Add BL0010 analyzer: Recommend InvokeVoidAsync over InvokeAsync<object>#64623
Add BL0010 analyzer: Recommend InvokeVoidAsync over InvokeAsync<object>#64623
Conversation
Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
… comparison Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a new Roslyn analyzer (BL0010) that detects usage of InvokeAsync<object> in JavaScript interop calls and recommends using InvokeVoidAsync instead. This addresses a common antipattern where developers use InvokeAsync<object> for JS calls that don't return a value, which can lead to serialization failures when JavaScript returns non-serializable values like window or undefined.
Key changes:
- New analyzer that flags
InvokeAsync<object>calls on IJSRuntime, IJSObjectReference, and their in-process variants - Comprehensive test coverage with 13 test cases covering various scenarios including extension methods
- Diagnostic set as a Warning with enabled-by-default behavior
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/Components/Analyzers/src/InvokeAsyncOfObjectAnalyzer.cs |
Implements the new BL0010 analyzer to detect InvokeAsync<object> calls on JS interop types |
src/Components/Analyzers/src/DiagnosticDescriptors.cs |
Adds the BL0010 diagnostic descriptor with Warning severity |
src/Components/Analyzers/src/Resources.resx |
Adds localized resource strings for the diagnostic title, format, and description |
src/Components/Analyzers/test/InvokeAsyncOfObjectAnalyzerTest.cs |
Provides comprehensive test coverage with 13 test cases for the analyzer |
| using Microsoft.CodeAnalysis.Operations; | ||
|
|
||
| namespace Microsoft.AspNetCore.Components.Analyzers; | ||
|
|
There was a problem hiding this comment.
Add #nullable enable directive after the namespace declaration to match the coding conventions used in other analyzers in this codebase. This ensures consistent nullable reference type handling across all analyzer files.
| #nullable enable |
There was a problem hiding this comment.
Added #nullable enable directive in commit 61397d9.
oroztocil
left a comment
There was a problem hiding this comment.
Are we sure the diagnostic will not fire on legitimate cases where the result is not thrown away, e.g.:
var res = await JS.InvokeAsync<object>("...");There should be tests for these cases.
@copilot add a test for this if there isn't one. If there is one point out the test name. |
Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
... Added test |
|
I see, I did not understand the reasoning from the description at first. |
ilonatommy
left a comment
There was a problem hiding this comment.
It's fine but I saw that there's something like CodeFixProvider, e.g. https://github.com/dotnet/aspnetcore/blob/main/src/Components/Analyzers/src/ComponentParametersShouldBePublicCodeFixProvider.cs.
It would automatically add "Fix this issue" proposal. What about adding it?
|
@ilonatommy code fixes don't work in Let's wait for adding a code fixer. |
|
/azp run aspnetcore-ci |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Add BL0010 analyzer: Recommend InvokeVoidAsync over InvokeAsync- You've read the Contributor Guide and Code of Conduct.
- You've included unit or integration tests for your change, where applicable.
- You've included inline docs for your change, where applicable.
- There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.
Adds Roslyn analyzer BL0010 to detect
InvokeAsync<object>and recommendInvokeVoidAsyncDescription
Using
InvokeAsync<object>for JS interop calls that don't return a value can cause serialization failures when JavaScript returns non-serializable values (e.g.,window). This analyzer catches the antipattern at compile time.Note: The diagnostic intentionally fires even when the result is assigned to a variable (e.g.,
var res = await JS.InvokeAsync<object>(...)), becauseobjectcannot be properly deserialized from JSON - the result will either be null or cause serialization errors if JavaScript returns a non-serializable value.Changes:
InvokeAsyncOfObjectAnalyzer.cs- DetectsInvokeAsync<object>onIJSRuntime,IJSObjectReference, and in-process variants (both direct and extension method calls)DiagnosticDescriptors.cs- Added BL0010 descriptor (Warning, enabled by default)Resources.resx- Localized diagnostic stringsInvokeAsyncOfObjectAnalyzerTest.cs- 14 unit testsExample flagged code:
Fixes #38549
Original prompt
InvokeVoidAsyncinstead ofInvokeAsync<object>#38549✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.